Skip to content

[perfmon] Fix typo in variable name #6700

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[perfmon] Fix typo in variable name #6700

wants to merge 1 commit into from

Conversation

gthvn1
Copy link
Contributor

@gthvn1 gthvn1 commented Oct 8, 2025

No description provided.

last-genius
last-genius approved these changes Oct 8, 2025
View reviewed changes
@gthvn1 gthvn1 force-pushed the gtn-fix-perfmon-typo branch from 1bb4376 to f9e5617 Compare October 8, 2025 15:49
@psafont
Copy link
Member

psafont commented Oct 9, 2025

@gthvn1 Tests are failing because perfmon doesn't have any tests. This means that currently perfmon can't be modified without spending the effort to add them

@gthvn1
Copy link
Contributor Author

gthvn1 commented Oct 9, 2025

Oh. I understood that python3/tests/test_extauth_hook_AD.py and nbd_client_manager:nbd_client_manager.py failed. So it was not related to my modification. And I see the coverage issue but I thought it was only a warning.
Ok so I guess that need to have a closer look to understand how to add a test ;)

@gthvn1
Copy link
Contributor Author

gthvn1 commented Oct 9, 2025

In fact I see in the logs:

...
python3/tests/test_perfmon.py::TestGetFsUsage::test_get_percent_fs_usage PASSED [ 40%]
python3/tests/test_perfmon.py::TestGetFsUsage::test_get_percent_log_fs_usage PASSED [ 41%]
python3/tests/test_perfmon.py::TestGetFsUsage::test_get_percent_log_fs_usage_same_file_system PASSED [ 42%]
python3/tests/test_perfmon.py::TestGetMemUsage::test_get_percent_mem_usage PASSED [ 43%]
python3/tests/test_perfmon.py::TestGetMemUsage::test_get_percent_mem_usage_exception PASSED [ 43%]
python3/tests/test_perfmon.py::TestGetPercentSRUsage::test_get_percent_sr_usage_correct_input PASSED [ 44%]
python3/tests/test_perfmon.py::TestGetPercentSRUsage::test_get_percent_sr_usage_exception_handling PASSED [ 45%]
...

So it looks like perfmon has some tests.
But when I run tests locally (I just run pytest tests) I have an issue with tests/observer/it_traces.py.

>           assert span.__name__ == "span_of_tracers"
E           AssertionError: assert '_span_noop' == 'span_of_tracers'
E
E             - span_of_tracers
E             + _span_noop

tests/observer/it_traces.py:73: AssertionError
---------------------------------------------------------------------------------------------- Captured stdout call ----------------------------------------------------------------------------------------------
Hello, I am a print() in traced_script.py.
----------------------------------------------------------------------------------------------- Captured log call ------------------------------------------------------------------------------------------------
DEBUG    __main__:observer.py:400 configs = ['/home/gthouvenin/git/xen-api/python3/tests/observer/observer.conf']
ERROR    __main__:observer.py:147 missing opentelemetry dependencies: No module named 'opentelemetry'
============================================================================================ short test summary info =============================================================================================
FAILED tests/observer/it_traces.py::it_creates_a_tracer - AssertionError: assert '_span_noop' == 'span_of_tracers'
========================================================================================= 1 failed, 109 passed in 0.32s ==========================================================================================

@lindig
Copy link
Contributor

lindig commented Oct 14, 2025

@snwoods is there anything related to tracing that could be outdated above?

@snwoods
Copy link
Contributor

snwoods commented Oct 14, 2025

The issue with the tests/observer/it_traces.py test is that you don't have the opentelemetry python module installed. The output shows missing opentelemetry dependencies which happens when there is an import error. It then falls back to the _span_noop function decorator, which is to say tracing is disabled. But the test expects span_of_tracers decorator function

@gthvn1
Copy link
Contributor Author

gthvn1 commented Oct 15, 2025 •
edited
Loading

after installing opentelemetry-api and opentelemetry-sdk I still have some issues:

>       assert msg[4].startswith("tracers=[<opentelemetry.sdk.trace.Tracer object at")
E       AssertionError: assert False
E        +  where False = <built-in method startswith of str object at 0x7f4128b5bad0>('tracers=[<opentelemetry.sdk.trace.Tracer object at')
E        +    where <built-in method startswith of str object at 0x7f4128b5bad0> = 'Overriding of current TracerProvider is not allowed'.startswith

tests/observer/it_traces.py:104: AssertionError
----------------------------------------------------------------------------------------------- Captured log call ------------------------------------------------------------------------------------------------
DEBUG    __main__:observer.py:400 configs = ['/home/gthouvenin/git/xen-api/python3/tests/observer/observer.conf']
DEBUG    __main__:observer.py:95 /home/gthouvenin/git/xen-api/python3/tests/observer/all.conf: {'module_names': 'XenAPI,tests.observer.traced_script'}
DEBUG    __main__:observer.py:155 module_names: ['XenAPI', 'tests.observer.traced_script']
DEBUG    __main__:observer.py:95 /home/gthouvenin/git/xen-api/python3/tests/observer/observer.conf: {'otel_resource_attributes': '"key1=value1,key2=value2"'}
WARNING  opentelemetry.trace:__init__.py:537 Overriding of current TracerProvider is not allowed
DEBUG    __main__:observer.py:267 tracers=[<opentelemetry.sdk.trace.Tracer object at 0x7f4126131d90>]
============================================================================================ short test summary info =============================================================================================
FAILED tests/observer/it_traces.py::it_creates_a_tracer - AssertionError: assert False
========================================================================================= 1 failed, 109 passed in 0.40s ==========================================================================================

Actually, my point was that on my laptop, all tests related to perfmon pass. I can try, but I think that if I just keep the comment in my PR, the CI tests will fail as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lindig lindig lindig approved these changes

@contificate contificate contificate approved these changes

@last-genius last-genius last-genius approved these changes

+1 more reviewer

@freddy77 freddy77 freddy77 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@gthvn1 @psafont @lindig @snwoods @freddy77 @contificate @last-genius